Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: show progress while waiting for framework server to start #4683

Merged
merged 24 commits into from
Jun 13, 2022
Merged

feat: show progress while waiting for framework server to start #4683

merged 24 commits into from
Jun 13, 2022

Conversation

ericapisani
Copy link
Contributor

@ericapisani ericapisani commented Jun 8, 2022 •
edited
Loading

Summary

Fixes #3288

From the issue linked above:

When we start a framework server (e.g. Gatsby) we try to connect to the default port of that framework.
However, if a user configures a custom port they should tell netlify dev about it.

The problem is that netlify dev times out after 10 minutes and does't show any kind of progress while waiting for the framework server to start, so the user thinks the CLI is stuck and has no way of knowing what the problem is.

Most users won't wait 10 minutes for the CLI to timeout.

The CLI should show some kind of progress while waiting to the framework server to start, detailing the port its trying to connect to and how to configure it.

Before video
https://user-images.githubusercontent.com/5655473/172719545-9a84bc50-1ffd-474a-8723-d947b1ba46e3.mov

After video
https://user-images.githubusercontent.com/5655473/172719578-6f06de6a-a45d-4f7b-9ddf-83478b139ddb.mov

Related documentation/notes

  • Available options for us in the wait-port package are only 'dots' or 'silent', related code can be found here
  • Determined it was the targetPort we were waiting for based on the netlify dev docs

For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)
Sea_Otter_(Enhydra_lutris)_(25169790524)_crop

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jun 8, 2022
@github-actions
Copy link

github-actions bot commented Jun 8, 2022 •
edited
Loading

📊 Benchmark results

Package size: 294 MB

^  294 MB 
│   ┌──┐  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
│   |▒▒|  
└───┴──┴──>
     T
Legend
  • T (current commit): 294 MB

@ericapisani ericapisani requested review from a team and removed request for a team June 8, 2022 21:28
@ericapisani ericapisani marked this pull request as draft June 8, 2022 21:34
@ericapisani ericapisani marked this pull request as ready for review June 10, 2022 13:25
ericapisani and others added 12 commits June 10, 2022 09:59
@ericapisani
test: add regex to remove extra dots that lead to flaky snapshots
79c07c2
@ericapisani
Merge branch 'ep/show-progress-while-waiting-for-framework-to-startup…
2ce0523 
…' of github.com:netlify/cli into ep/show-progress-while-waiting-for-framework-to-startup
@ericapisani @github-actions
chore: update contributors field
52c1edc
@token-generator-app @github-actions
chore: update contributors field
fc5bcad
@ericapisani
test: move normalizer into the framework test suite
3497c30 
the regex being applied was affecting other tests
@ericapisani
Merge branch 'ep/show-progress-while-waiting-for-framework-to-startup…
a00dc1a 
…' of github.com:netlify/cli into ep/show-progress-while-waiting-for-framework-to-startup
@ericapisani @github-actions
chore: update contributors field
a4c5a7d
@ericapisani
Merge branch 'main' into ep/show-progress-while-waiting-for-framework…
136a1c1 
…-to-startup
@ericapisani
test: silly mistake
ba2f5dd
@ericapisani
Merge branch 'ep/show-progress-while-waiting-for-framework-to-startup…
4699f96 
…' of github.com:netlify/cli into ep/show-progress-while-waiting-for-framework-to-startup
@ericapisani
style: lint
c47224f
@ericapisani
test: update other tests to use the other normalizer func
6bb3d10
@ericapisani ericapisani requested a review from a team June 10, 2022 15:24
nickytonline
nickytonline approved these changes Jun 10, 2022
View reviewed changes
Copy link
Contributor

@nickytonline nickytonline left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and works great!

CleanShot.2022-06-10.at.15.54.18.mp4

@ericapisani ericapisani added the automerge Add to Kodiak auto merge queue label Jun 13, 2022
@kodiakhq kodiakhq bot merged commit 33c931b into main Jun 13, 2022
@kodiakhq kodiakhq bot deleted the ep/show-progress-while-waiting-for-framework-to-startup branch June 13, 2022 14:45
@token-generator-app token-generator-app bot mentioned this pull request Jun 13, 2022
Merged
@danez
Copy link
Contributor

danez commented Jun 17, 2022 •
edited
Loading

We have some random test failures now, that sometimes contain a . and I think this comes from this PR.

https://github.com/netlify/cli/runs/6925957490?check_suite_focus=true

Can you have a look at why this is happening?

@nickytonline
Copy link
Contributor

@danez, @ericapisani is currently on PTO for the next little while. I know she had put something in place to make the snapshot tests consistent with the dots. How big a blocker is it at the moment? Is it occasional or pretty frequent the failures?

@danez
Copy link
Contributor

danez commented Jun 17, 2022

Ah okay, then i will see if i can figure something out maybe. It is not super urgent, nothing that can't wait a little.

@danez danez mentioned this pull request Jun 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahetter sarahetter sarahetter approved these changes

@nickytonline nickytonline nickytonline approved these changes

Assignees
No one assigned
Labels
automerge Add to Kodiak auto merge queue type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: show progress while waiting for framework server to start
4 participants
@ericapisani @danez @nickytonline @sarahetter